Skip to content

Refactor chat into a composable workspace shell#2011

Open
juliusmarminge wants to merge 2 commits intomainfrom
t3code/composable-chat-layout
Open

Refactor chat into a composable workspace shell#2011
juliusmarminge wants to merge 2 commits intomainfrom
t3code/composable-chat-layout

Conversation

@juliusmarminge
Copy link
Copy Markdown
Member

@juliusmarminge juliusmarminge commented Apr 14, 2026

CleanShot.2026-04-13.at.19.42.25.mp4
CleanShot.2026-04-13.at.21.41.01.mp4

Summary

  • Reworked chat UI around a new workspace shell so thread surfaces, terminal surfaces, and route sync are managed through shared workspace state.
  • Moved terminal launch/open/close behavior into workspace-aware hooks and stores, reducing chat view responsibilities.
  • Expanded the command palette and keybinding handling to support workspace-specific actions and spatial layout targets.
  • Added persistence and tests for workspace documents, plus store coverage for the new workspace model.

Testing

  • Not run (not provided in the commit diff).
  • Added/updated unit tests for client persistence storage and workspace store behavior.
  • Project requirements still call for bun fmt, bun lint, bun typecheck, and bun run test before merge.

Note

High Risk
Large UI refactor that changes chat routing, focus/keyboard handling, and terminal rendering/persistence; regressions could affect navigation, shortcuts, and local workspace state across sessions.

Overview
Refactors the chat UI to run inside a new workspace shell that manages multi-pane split layout, tabbed surfaces, drag/drop placement, and route-to-focused-surface syncing instead of rendering ChatView directly from routes.

Moves terminal UI from an in-thread drawer to a dedicated terminal surface (ThreadTerminalSurface) controlled by workspace state, and updates ChatView/shortcuts to toggle and launch terminals via workspace APIs.

Expands keybindings and command palette to support workspace-level commands (pane split/close/zoom, focus navigation, resize/move, terminal open variants), adjusts defaults (e.g., diff.toggle to mod+alt+d), and adds sidebar actions/drag support to open threads in new tabs or splits.

Adds localStorage persistence helpers and tests for saving/loading the workspace document, plus small focus/perf fixes in composer/editor interactions to behave correctly with pane activation.

Reviewed by Cursor Bugbot for commit 5c8504d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Refactor chat routes into a composable workspace shell with split-pane and drag-and-drop support

  • Introduces a workspace store (store.ts) managing thread/terminal surfaces, split-pane windows, focus, and debounced localStorage persistence, replacing per-route ChatView rendering with surface-based lifecycle management.
  • Chat routes (_chat.$environmentId.$threadId.tsx, _chat.draft.$draftId.tsx) no longer render ChatView directly; instead they call openThreadSurface to register surfaces in the workspace, and the new WorkspaceShell renders all active surfaces.
  • Adds workspace commands (pane split/close/zoom, focus navigation, resize, equalize, tab move) to the command palette and global keyboard handling, with new default keybindings; diff.toggle moves from mod+d to mod+alt+d.
  • Enables drag-and-drop for sidebar thread rows and adds 'Open in new tab' / 'Open in split right/down' context menu actions.
  • WorkspaceRouteSync keeps the focused workspace surface and the router URL in sync bidirectionally.
  • Risk: diff.toggle keybinding changes for all users; terminal drawer visibility is now controlled by workspace-level state rather than component-local state, which may affect existing terminal session behavior.

Macroscope summarized 5c8504d.

- Add workspace document storage and store tests
- Route terminal and command-palette actions through workspace surfaces
- Update chat layout, sidebar, and thread terminal handling
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e9e35d48-d15a-4415-9d96-c39240e2a868

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/composable-chat-layout

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Apr 14, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for all 4 issues found in the latest run.

  • ✅ Fixed: Unused state variable nowTick added to ChatView
    • Removed the unused nowTick and setNowTick state declaration from ChatView.tsx as it was never referenced anywhere in the component.
  • ✅ Fixed: WorkspaceRouteSync component exported but never used
    • Deleted the entire WorkspaceRouteSync.tsx file since the component was never imported or rendered anywhere in the codebase.
  • ✅ Fixed: Unused terminalFocusRequestId state after refactor
    • Removed the unused terminalFocusRequestId and setTerminalFocusRequestId state declaration from ChatView.tsx as all consumers were removed in a prior refactor.
  • ✅ Fixed: Sidebar thread context menu navigates after split open
    • Removed the redundant router.navigate() calls after openThreadInSplit/openThreadInNewTab in the sidebar context menu handlers, which were causing the route component's useEffect to call openThreadSurface with focus-or-tab disposition and potentially refocus the surface into the wrong pane.

Create PR

Or push these changes by commenting:

@cursor push dc4ee0b932
Preview (dc4ee0b932)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -498,8 +498,6 @@
   // When set, the thread-change reset effect will open the sidebar instead of closing it.
   // Used by "Implement in a new thread" to carry the sidebar-open intent across navigation.
   const planSidebarOpenOnNextThreadRef = useRef(false);
-  const [terminalFocusRequestId, setTerminalFocusRequestId] = useState(0);
-  const [nowTick, setNowTick] = useState(() => Date.now());
   const [pullRequestDialogState, setPullRequestDialogState] =
     useState<PullRequestDialogState | null>(null);
   const [attachmentPreviewHandoffByMessageId, setAttachmentPreviewHandoffByMessageId] = useState<

diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -1654,28 +1654,16 @@
 
       if (clicked === "open-new-tab") {
         openThreadInNewTab(serverThreadSurfaceInput(threadRef));
-        void router.navigate({
-          to: "/$environmentId/$threadId",
-          params: buildThreadRouteParams(threadRef),
-        });
         return;
       }
 
       if (clicked === "open-split-right") {
         openThreadInSplit(serverThreadSurfaceInput(threadRef), "x");
-        void router.navigate({
-          to: "/$environmentId/$threadId",
-          params: buildThreadRouteParams(threadRef),
-        });
         return;
       }
 
       if (clicked === "open-split-down") {
         openThreadInSplit(serverThreadSurfaceInput(threadRef), "y");
-        void router.navigate({
-          to: "/$environmentId/$threadId",
-          params: buildThreadRouteParams(threadRef),
-        });
         return;
       }
 
@@ -1729,7 +1717,6 @@
       openThreadInNewTab,
       openThreadInSplit,
       project.cwd,
-      router,
     ],
   );
 

diff --git a/apps/web/src/components/workspace/WorkspaceRouteSync.tsx b/apps/web/src/components/workspace/WorkspaceRouteSync.tsx
deleted file mode 100644
--- a/apps/web/src/components/workspace/WorkspaceRouteSync.tsx
+++ /dev/null
@@ -1,130 +1,0 @@
-import { useLocation, useNavigate, useParams } from "@tanstack/react-router";
-import { useEffect, useMemo, useRef } from "react";
-
-import { useComposerDraftStore } from "../../composerDraftStore";
-import { resolveThreadRouteTarget } from "../../threadRoutes";
-import { useFocusedWorkspaceRouteTarget, useWorkspaceStore } from "../../workspace/store";
-import type { ThreadSurfaceInput } from "../../workspace/types";
-
-function sameRouteTarget(
-  left: ReturnType<typeof resolveThreadRouteTarget>,
-  right: ReturnType<typeof resolveThreadRouteTarget>,
-): boolean {
-  if (!left && !right) {
-    return true;
-  }
-  if (!left || !right || left.kind !== right.kind) {
-    return false;
-  }
-  if (left.kind === "server" && right.kind === "server") {
-    return (
-      left.threadRef.environmentId === right.threadRef.environmentId &&
-      left.threadRef.threadId === right.threadRef.threadId
-    );
-  }
-  if (left.kind === "draft" && right.kind === "draft") {
-    return left.draftId === right.draftId;
-  }
-  return false;
-}
-
-export function WorkspaceRouteSync() {
-  const navigate = useNavigate();
-  const openThreadSurface = useWorkspaceStore((state) => state.openThreadSurface);
-  const currentRouteTarget = useParams({
-    strict: false,
-    select: (params) => resolveThreadRouteTarget(params),
-  });
-  const focusedRouteTarget = useFocusedWorkspaceRouteTarget();
-  const pathname = useLocation({
-    select: (location) => location.pathname,
-  });
-  const previousPathnameRef = useRef(pathname);
-  const draftSession = useComposerDraftStore((store) =>
-    currentRouteTarget?.kind === "draft" ? store.getDraftSession(currentRouteTarget.draftId) : null,
-  );
-  const currentRouteSurfaceInput = useMemo<ThreadSurfaceInput | null>(() => {
-    if (!currentRouteTarget) {
-      return null;
-    }
-    if (currentRouteTarget.kind === "server") {
-      return {
-        scope: "server",
-        threadRef: currentRouteTarget.threadRef,
-      };
-    }
-    if (!draftSession) {
-      return null;
-    }
-    return {
-      scope: "draft",
-      draftId: currentRouteTarget.draftId,
-      environmentId: draftSession.environmentId,
-      threadId: draftSession.threadId,
-    };
-  }, [currentRouteTarget, draftSession]);
-
-  useEffect(() => {
-    const pathnameChanged = previousPathnameRef.current !== pathname;
-    previousPathnameRef.current = pathname;
-
-    if (currentRouteTarget) {
-      if (!currentRouteSurfaceInput) {
-        return;
-      }
-
-      if (!focusedRouteTarget || pathnameChanged) {
-        openThreadSurface(currentRouteSurfaceInput, "focus-or-tab");
-        return;
-      }
-
-      if (sameRouteTarget(currentRouteTarget, focusedRouteTarget)) {
-        return;
-      }
-
-      void navigateToRouteTarget(navigate, focusedRouteTarget);
-      return;
-    }
-
-    if (!focusedRouteTarget) {
-      return;
-    }
-
-    void navigateToRouteTarget(navigate, focusedRouteTarget);
-  }, [
-    currentRouteSurfaceInput,
-    currentRouteTarget,
-    focusedRouteTarget,
-    navigate,
-    openThreadSurface,
-    pathname,
-  ]);
-
-  return null;
-}
-
-function navigateToRouteTarget(
-  navigate: ReturnType<typeof useNavigate>,
-  target: NonNullable<ReturnType<typeof resolveThreadRouteTarget>>,
-) {
-  if (target.kind === "server") {
-    return navigate({
-      to: "/$environmentId/$threadId",
-      params: {
-        environmentId: target.threadRef.environmentId,
-        threadId: target.threadRef.threadId,
-      },
-      replace: true,
-      search: {},
-    });
-  }
-
-  return navigate({
-    to: "/draft/$draftId",
-    params: {
-      draftId: target.draftId,
-    },
-    replace: true,
-    search: {},
-  });
-}
\ No newline at end of file

You can send follow-ups to the cloud agent here.

// Used by "Implement in a new thread" to carry the sidebar-open intent across navigation.
const planSidebarOpenOnNextThreadRef = useRef(false);
const [terminalFocusRequestId, setTerminalFocusRequestId] = useState(0);
const [nowTick, setNowTick] = useState(() => Date.now());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused state variable nowTick added to ChatView

Low Severity

The state variable nowTick and its setter setNowTick are declared but never read or called anywhere in the component. This appears to be accidentally committed scaffolding for a feature that was never wired up, adding dead code to an already large component.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d3d096c. Configure here.

replace: true,
search: {},
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WorkspaceRouteSync component exported but never used

Low Severity

WorkspaceRouteSync is a fully implemented component (130 lines) that is exported but never imported or rendered anywhere in the codebase. The route-to-workspace synchronization it would perform is instead duplicated across WorkspaceRouteFallback in WorkspaceShell.tsx and the individual route view components. This is dead code that adds maintenance burden.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d3d096c. Configure here.

params: buildThreadRouteParams(threadRef),
});
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar thread context menu navigates after split open

Medium Severity

The "Open in split right/down" and "Open in new tab" context menu handlers call openThreadInSplit/openThreadInNewTab to create a new workspace pane, then immediately navigate the route to that thread. The navigation triggers the route view's useEffect which calls openThreadSurface with "focus-or-tab" disposition, potentially refocusing the surface into the original pane instead of the newly created split, undermining the user's intent to keep both panes visible.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d3d096c. Configure here.

if (!activeThreadRef) {
return;
}
storeSetTerminalLaunchContext(activeThreadRef, {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low components/ChatView.tsx:1390

When a draft thread's environment is changed via onEnvironmentChange, routeThreadRef (using route environmentId) diverges from activeThreadRef (using draftThread.environmentId). The terminal launch context is written and cleared using activeThreadRef (lines 1390, 1851, 1868) but read using routeThreadRef at line 531. This causes the clearing logic to target a different store key than the reading logic, leaving the launch context orphaned and never visible to the component that needs it.

-      storeSetTerminalLaunchContext(activeThreadRef, {
+      storeSetTerminalLaunchContext(routeThreadRef, {
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around line 1390:

When a draft thread's environment is changed via `onEnvironmentChange`, `routeThreadRef` (using route `environmentId`) diverges from `activeThreadRef` (using `draftThread.environmentId`). The terminal launch context is written and cleared using `activeThreadRef` (lines 1390, 1851, 1868) but read using `routeThreadRef` at line 531. This causes the clearing logic to target a different store key than the reading logic, leaving the launch context orphaned and never visible to the component that needs it.

Evidence trail:
- apps/web/src/components/ChatView.tsx lines 403-406: routeThreadRef definition uses route environmentId
- apps/web/src/components/ChatView.tsx lines 572-575: activeThreadRef definition uses activeThread.environmentId
- apps/web/src/components/ChatView.tsx line 564: activeThread = localDraftThread for draft threads
- apps/web/src/components/ChatView.tsx lines 1239-1250: onEnvironmentChange calls setDraftThreadContext but does not navigate
- apps/web/src/composerDraftStore.ts lines 1919-1963: setDraftThreadContext updates draftThread.environmentId
- apps/web/src/components/ChatView.tsx line 1390: storeSetTerminalLaunchContext(activeThreadRef, ...)
- apps/web/src/components/ChatView.tsx line 532: reads from scopedThreadKey(routeThreadRef)
- apps/web/src/components/ChatView.tsx lines 1851-1853, 1868-1870: storeClearTerminalLaunchContext(activeThreadRef)
- apps/web/src/terminalStateStore.ts lines 264-266: terminalThreadKey calls scopedThreadKey
- packages/client-runtime/src/scoped.ts lines 20-22: scopedRefKey produces `{environmentId}:{localId}` keys

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp bot commented Apr 14, 2026

Approvability

Verdict: Needs human review

1 blocking correctness issue found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR.

You can customize Macroscope's approvability policy. Learn more.

- add workspace pane focus, split, resize, and zoom commands
- wire chat composer focus repair across workspace activation
- enable dragging threads into workspace surfaces
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 4 total unresolved issues (including 3 from previous reviews).

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Workspace commands bypass command palette open check
    • Moved the useCommandPaletteStore.getState().open guard to run immediately after resolving the shortcut command, before both the workspace command dispatch and standalone terminal command handling.

Create PR

Or push these changes by commenting:

@cursor push fc52f7f4cf
Preview (fc52f7f4cf)
diff --git a/apps/web/src/routes/_chat.tsx b/apps/web/src/routes/_chat.tsx
--- a/apps/web/src/routes/_chat.tsx
+++ b/apps/web/src/routes/_chat.tsx
@@ -42,6 +42,10 @@
           terminalOpen,
         },
       });
+      if (useCommandPaletteStore.getState().open) {
+        return;
+      }
+
       const isFocusedStandaloneTerminal = focusedWorkspaceSurface?.kind === "terminal";
       if (command && isWorkspaceCommandId(command)) {
         event.preventDefault();
@@ -73,10 +77,6 @@
         }
       }
 
-      if (useCommandPaletteStore.getState().open) {
-        return;
-      }
-
       if (event.key === "Escape" && selectedThreadKeysSize > 0) {
         event.preventDefault();
         clearSelection();

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 5c8504d. Configure here.

event.stopPropagation();
void executeWorkspaceCommand(command);
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workspace commands bypass command palette open check

Medium Severity

In ChatRouteGlobalShortcuts, workspace commands (via isWorkspaceCommandId) are dispatched before the useCommandPaletteStore.getState().open guard on line 76. This means shortcuts like Mod+W (workspace.pane.close), Mod+[/Mod+] (focus previous/next), and resize/move commands will execute even while the command palette dialog is open. The palette check needs to come before the workspace command dispatch, consistent with how other commands are guarded.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5c8504d. Configure here.

Comment on lines +1010 to +1022
return {
document: {
...document,
rootNodeId: nextTree.rootNodeId,
nodesById: nextTree.nodesById,
windowsById: nextWindowsById,
focusedWindowId: fallbackWindowId,
mobileActiveWindowId: fallbackWindowId,
},
sourceWindowId: located.windowId,
surface,
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High workspace/store.ts:1010

When a window is removed because its last surface was detached, focusedWindowId and mobileActiveWindowId are unconditionally overwritten with fallbackWindowId. If the removed window was not the currently focused window, this incorrectly shifts focus away from the user's actual focused window.

  return {
    document: {
      ...document,
      rootNodeId: nextTree.rootNodeId,
      nodesById: nextTree.nodesById,
      windowsById: nextWindowsById,
-      focusedWindowId: fallbackWindowId,
-      mobileActiveWindowId: fallbackWindowId,
+      focusedWindowId:
+        document.focusedWindowId === located.windowId
+          ? fallbackWindowId
+          : document.focusedWindowId,
+      mobileActiveWindowId:
+        document.mobileActiveWindowId === located.windowId
+          ? fallbackWindowId
+          : document.mobileActiveWindowId,
    },
    sourceWindowId: located.windowId,
    surface,
  };
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/workspace/store.ts around lines 1010-1022:

When a window is removed because its last surface was detached, `focusedWindowId` and `mobileActiveWindowId` are unconditionally overwritten with `fallbackWindowId`. If the removed window was not the currently focused window, this incorrectly shifts focus away from the user's actual focused window.

Evidence trail:
apps/web/src/workspace/store.ts lines 963-1022 (function `detachSurfaceFromWindow`), specifically lines 1005-1022 where `focusedWindowId: fallbackWindowId` and `mobileActiveWindowId: fallbackWindowId` are set unconditionally without checking if `located.windowId === document.focusedWindowId`. Compare with how the same pattern exists in `closeSurfaceById` at lines 917-924.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant